-
Notifications
You must be signed in to change notification settings - Fork 72
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
remove use of empty var QT_LIBRARIES and use VTK_LIBRARIES and ITK_LIBRARIES… #610
base: master
Are you sure you want to change the base?
Conversation
ITKIOTransformMatlab ITKIOMRC ITKIOTransformInsightLegacy ITKRegistrationCommon | ||
ITKIOMeta ITKStatistics ITKIOPhilipsREC) | ||
|
||
#ITKCommon ITKTransformFactory |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to remove
…nstead of set_definition, removed unecessary link, use target_include_directories instead of include_directories
dtkCoreSupport | ||
dtkLog | ||
medCore | ||
medCoreLegacy | ||
ITKCommon |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i'm curious, why do you prefer them in the find_package than here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like to put them in the find required section. And I hope that one time I could remove most of them .
target_compile_definitions(${TARGET_NAME} PUBLIC ${TARGET_NAME_UP}_VERSION="${${TARGET_NAME}_VERSION}") | ||
|
||
## ############################################################################# | ||
## include directorie. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
*directories
|
||
|
||
## ############################################################################# | ||
## add library | ||
## include directorie. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
*directories (there are some changes like that in the PR)
find_package(VTK REQUIRED COMPONENTS vtkIOImage) | ||
include(${VTK_USE_FILE}) | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can remove this empty line
target_compile_definitions(${TARGET_NAME} PUBLIC ${TARGET_NAME_UP}_VERSION="${${TARGET_NAME}_VERSION}") | ||
|
||
## ############################################################################# | ||
## include directorie. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
directories
I think there is a problem with the PR @nbaraka you lost all your changes except the last commit :/ |
…de_directories, done for few vtk , still some remains
…de(VTK_file) but have to add VTK_DEFINITIONS; may have to add some ITK_DEFINITONS
…n project, set add_library kind to be module for plugins : seems to define better what it is planned to compile than shared, but it is not true for plugins that will be linked with other plugins
…arget_compile_definitions in most cmake files, wip for definition
It's really cool to improve cmake mechanism and promote "modern cmake" philosophy. |
To link to specific qt lib : look for qt package with the required lib (for example qtsql).